Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a function to search the time_ranges of the catalog #291

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wachsylon
Copy link

Hi,
I am a scientific programmer at DKRZ and I really appreciate intake and intake-esm for the purpose of finding and accessing data.

An issue related to the subsetting in time ranges in the time_range column of CMIP catalogs is that the length of the string values vary. This code of this PR allows to search for all data that covers a time range defined by two time strings.
We think that this will be very beneficial when working with CMIP catalogs.

A colleague of mine (Ludwig Lierhammer) has written this code snippet and asked me to share it.
I think the best place to implement that would be in your software in search.py so I tried to implement it there.
I tested the function itself however I do not know how to best call it in the intake workflow. Therefore, I do not have a test.

I am a beginner in python software managing. I try to follow your steps but I do also have not much time for caring for all the pull request tests especially since this code snippet needs some adaption.

And, an additonal comment:
I think that these 10 steps for a simple contribution to the repo takes too much time for an external programmer who has not much experience. This does not help if it is your intention to motivate the community to contribute to the repo. Idk how other repos handle it and I do understand that it is a part of guaranteeing stable code. For me, it is frustrating.
Cant that be done by automized CI tests?

Best regards,
Fabi

@wachsylon wachsylon requested a review from andersy005 as a code owner October 2, 2020 11:10
@andersy005
Copy link
Member

@wachsylon, Thank you for your patience and putting this PR together! Since it's been a while since you submitted this PR, I was wondering if this is something you are still interested in?

I am asking because I think this is very useful functionality and would like to work with you to make it generic enough (the current implementation is a bit specific and tends to cover a limited number of use cases).

@wachsylon
Copy link
Author

wachsylon commented Jan 4, 2021

Hi @andersy005 ,
ok, we could try.

this is very useful functionality

cool.

would like to work with you to make it generic enough

What do you have in mind?

Best,
Fabi

Base automatically changed from master to main January 28, 2021 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants